Skip to content

feat(clerk-js): Add support for loading UI styles as first CSS stylesheet - #4291

Merged
dstaley merged 4 commits into
brk.feat/render-ui-in-clerk-jsfrom
ds.feat/dynamic-styles
Oct 7, 2024
Merged

feat(clerk-js): Add support for loading UI styles as first CSS stylesheet#4291
dstaley merged 4 commits into
brk.feat/render-ui-in-clerk-jsfrom
ds.feat/dynamic-styles

Conversation

@dstaley

@dstaley dstaley commented Oct 4, 2024

Copy link
Copy Markdown
Member

Description

This PR adds support for inserting the new UI stylesheet as the first stylesheet in the document. This is to allow user styles to take precedence over our own. For example:

// @clerk/ui/styles.css
.cl-button { display: flex; }
// app/style.css
.hidden { display: none; }

If the UI stylesheet appears after the app stylesheet in the DOM, cl-button will override hidden given they have the same specificity. By prepending our stylesheet, we allow our users' styles to take precedence.

To support this, I had to remove Webpack's experimental support for CSS which doesn't currently support emitting the URL of the stylesheet; it always wants to append it to the DOM.

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

@changeset-bot

changeset-bot Bot commented Oct 4, 2024

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 4380da2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dstaley dstaley changed the title feat: Add support for loading UI styles as first CSS stylesheet feat(clerk-js): Add support for loading UI styles as first CSS stylesheet Oct 4, 2024
import type { ClerkHostRouter } from '@clerk/shared/router';
import { ClerkHostRouterContext } from '@clerk/shared/router';
import type { ClerkOptions, LoadedClerk } from '@clerk/types';
//@ts-ignore - This is treated as a string export by Webpack

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[question] Would it be better to use@ts-expect-error so in the case this is no longer an issue in the future the @ts-ignore doesn't just stick around forever?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted this file as it wasn't being included anyway, only the src/ directory was.

rootElement.setAttribute('id', 'clerk-components');
document.body.appendChild(rootElement);

const stylesheet = document.createElement('link');

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brkalow I'm assuming since we're within an instance where the rootElement didn't exist that we can safely assume the stylesheet isn't part of the DOM. Do you think we need to add some attributes to the link element and check for those?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dstaley It should be safe, but in this case I feel like this is a place where we it would be a good idea to add the check.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added! 4380da2

@brkalow brkalow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@dstaley
dstaley merged commit 74c81bf into brk.feat/render-ui-in-clerk-js Oct 7, 2024
@dstaley
dstaley deleted the ds.feat/dynamic-styles branch October 7, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants